-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ref: Convert React and Vue Tracing to use active transaction #2741
Conversation
export function getActiveTransaction<T extends Transaction>(hub: HubType): T | undefined { | ||
if (hub && hub.getScope) { | ||
const scope = hub.getScope() as Scope; | ||
if (scope) { | ||
return scope.getTransaction() as T | undefined; | ||
} | ||
} | ||
|
||
return undefined; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I don't like that we have this duplicated everywhere :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't doing hub.getScope().getTransaction()
the same?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public getScope(): Scope | undefined
sentry-javascript/packages/hub/src/hub.ts
Line 140 in e8a37d2
public getScope(): Scope | undefined { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it is more like hub?.getScope?.getTransaction()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can always refactor this if we figure out a better way to do this.
* ref: Make React Profiler use active transaction * ref: Make Vue Tracing using active transaction
* feat: Create IdleTransaction class (#2720) * feat: Add BrowserTracing integration (#2723) * feat: Add span creators to @sentry/tracing package (#2736) * ref: Convert React and Vue Tracing to use active transaction (#2741) * build: generate tracing bundles * fix: Remove circular dependency between span and transaction * ref: Add side effects true to tracing * build: Only include @sentry/browser for bundle * fix: Make sure vue and react are backwards compatible with @sentry/apm
Last PR before #2719 is ready :)
This one changes both React and Vue to add children to the current active transaction instead of pushing and popping activities.
This is a change in behaviour, as if the components no longer add/remove activities, the 0 activity point is reached more quickly by the
Tracing
integration, but I figured if we are looking to move past it, we should just do this.I updated the react tests and tested on Sentry and everything seems good.